TLS_PASSTHROUGH document #650

zijun726911 · 2024-06-10T20:37:24Z

What type of PR is this? Add TLS_PASSTHROUGH feature documentation

The initial doc cherry picked from https://github.com/liwenwu-amazon/aws-application-networking-k8-publics/tree/tls-route-support-mar20

main branch to liwenwu-amazon:tls-route-support-mar20 branch diff: zijun726911#3

Changes

Add TLS_PASSTHROUGH user guides doc
Add TLSRoute API reference
Change the target group policy doc, mention it can targetRef to a ServiceExport
Make main and publish-doc branch git commit trigger different doc version mike deploy

Test

Following the docs/guides/tls-passthrough.md step by step, it can set up the single and multi cluster use cases TLS_PASSTHROUGH e2e connectivity
Testing for .github/workflows/publish-doc.yaml:

Used my own repo, main and publish-doc branch git commit can trigger public-doc and then pages-build-deployment workflow actions, https://github.com/zijun726911/aws-application-networking-k8s/actions

It can build the doc version dev for main branch git commit and build v1.9.9 for release-v1.9.9 branch git commit

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

zijun726911 · 2024-06-11T17:48:28Z

helm/crds/gateway.networking.k8s.io_tlsroutes.yaml

@@ -0,0 +1,894 @@
+apiVersion: apiextensions.k8s.io/v1


I found we need to add that CRD under the folder helm/crds/ ,otherwise the built helm chart don't include that (that's the reason why e2e-test github action failed previously)

Mengli-Shu · 2024-06-12T06:00:17Z

docs/api-types/tls-route.md

+### Considerations
+
+- `TLSRoute` sectionName must refer to an `TLS` protocol listener with `mode: Passthrough` in the parentRefs `Gateway`.
+- `TLSRoute` only supports to have one rule.


perhaps to be more specific, only supports one default rule

But I think Kubernetes resource TLSRoute don't have a concept of "default rule" ?
https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1alpha2.TLSRoute

ok, i misunderstood. or the wording can be "can only have one rule"

Probably change to: " the TLSRoute managed by aws gateway api controller only supports to have one rule."

Since the k8s gateway api standard supports to have multiple rules, and other gateway api implementation can have multiple rules in TLSRoute for example: envoyproxy/gateway, https://github.com/envoyproxy/gateway

But our implementation don't support that because we are base on vpc lattice

In other docs we have a "Features" and a "Limitations" section. Can we be consistent here as well?

I did consider to add "Features" and "Limitations"， but don't think TLSRoute has some fancy features that need one separate section besides some words in the "Introduction" and " Example Configuration" sections.

For "Limitations", just use the word "Considerations", same as the word in the vpc lattice doc because that are more likely design decision and not limitation? What do you think?

OK I think it's fine to leave as-is

docs/api-types/tls-route.md

docs/guides/tls-passthrough.md

Mengli-Shu · 2024-06-12T06:22:48Z

files/examples/tls-rate2-targetgrouppolicy.yaml

+        name: tls-rate2
+    protocol: TCP
+    healthCheck:
+         enabled: false


do we want to include an example with health check enabled?

The new example nginx image: public.ecr.aws/x2j8p8w7/https-server:latest has some issue to enable health check. the targets are healthy initially but it will then change to unhealthy. Still plan to keep the healthcheck disable as unhealthy target will be weighted out and traffic will not work.

But in our other TargetGroupPolicy, it did have healthcheck enabled examples:

aws-application-networking-k8s/docs/api-types/target-group-policy.md

Lines 36 to 62 in 112b6b0

## Example Configuration

This will enable HTTPS traffic between the gateway and Kubernetes service, with customized health check configuration.

```

apiVersion: application-networking.k8s.aws/v1alpha1

kind: TargetGroupPolicy

metadata:

name: test-policy

spec:

targetRef:

group: ""

kind: Service

name: my-parking-service

protocol: HTTPS

protocolVersion: HTTP1

healthCheck:

enabled: true

intervalSeconds: 5

timeoutSeconds: 1

healthyThresholdCount: 3

unhealthyThresholdCount: 2

path: "/healthcheck"

port: 80

protocol: HTTP

protocolVersion: HTTP1

statusMatch: "200"

erikfuller

Thanks for putting this together. Noticed a few typos and asked a few questions.

erikfuller · 2024-06-13T19:50:12Z

.github/workflows/publish-doc.yaml

+            # Deploy to the mike doc version `dev` and update the `latest` alias for the main branch new git commits
+            mike deploy dev latest --update-aliases --push
+            mike set-default latest
+          elif [[ ${{ github.ref }} == refs/heads/release-v* ]]; then


this'll be super helpful

docs/api-types/tls-route.md

erikfuller · 2024-06-13T20:10:08Z

docs/api-types/tls-route.md

+### Considerations
+
+- `TLSRoute` sectionName must refer to an `TLS` protocol listener with `mode: Passthrough` in the parentRefs `Gateway`.
+- `TLSRoute` only supports to have one rule.


In other docs we have a "Features" and a "Limitations" section. Can we be consistent here as well?

docs/api-types/tls-route.md

erikfuller · 2024-06-13T21:09:56Z

docs/api-types/tls-route.md

+- `TLSRoute` sectionName must refer to an `TLS` protocol listener with `mode: Passthrough` in the parentRefs `Gateway`.
+- `TLSRoute` only supports to have one rule.
+- `TLSRoute` doesn't support any rule matching condition.
+- The `hostnames` field with exactly one host name is required. This domain name is used as a vpc lattice's Service Name Indication (SNI) match to route the traffic to the correct backend service.


Is this essential? "This domain name is used as a vpc lattice's Service Name Indication (SNI) match to route the traffic to the correct backend service."

We explain this later with "The customer must use this domain name..." which I think is more helpful

Ok, make sense to remove, just hope to emphasize, customer cannot use lattice generated DNS name to send traffic to backend service, they must use custom domain name

Makes sense. I think we need to lean on the public Lattice documentation as well. The docs we have here in the controller don't have to be comprehensive and should focus on the integration rather than the core functionality Lattice provides.

erikfuller · 2024-06-13T21:16:10Z

docs/guides/tls-passthrough.md

+### 1. Configure TLS Passthrough Listener on Gateway
+
+```
+kubectl apply -f files/examples/gateway-tls-passthrough.yaml


I don't see this file, should it be "my-gateway-tls...."?

Good catch, yes, should be my-gateway-tls-passthrough.yaml

erikfuller · 2024-06-13T21:22:39Z

docs/guides/tls-passthrough.md

+* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
+* ALPN, server accepted to use h2
+* Server certificate:
+*  subject: C=US; ST=wa; L=seattle; O=aws; OU=lattice; CN=liwen.ssl-test.com; [email protected]


Can we scrub this email address?

erikfuller · 2024-06-13T21:23:31Z

docs/guides/tls-passthrough.md

+* Trying 169.254.171.0:443...
+* Connected to nginx-test.my-test.com (169.254.171.0) port 443 (#0)
+* ALPN, offering h2
+* ALPN, offering http/1.1
+* Cipher selection: ALL:!EXPORT:!EXPORT40:!EXPORT56:!aNULL:!LOW:!RC4:@STRENGTH
+* successfully set certificate verify locations:
+*  CAfile: /etc/pki/tls/certs/ca-bundle.crt
+*  CApath: none


Is all this important for people reading the docs? Should we rather highlight individual lines if we think showing the debug output is crucial?

Shorten that part in the new revision

docs/guides/tls-passthrough.md

erikfuller

LGTM. Thanks for putting this together!

TLS_PASSTHROUGH document

946a64f

zijun726911 force-pushed the tls-passthrough-doc-2 branch from c34d9d4 to 946a64f Compare June 11, 2024 16:52

Minor Change

b144147

zijun726911 changed the title ~~[Draft] TLS_PASSTHROUGH document~~ TLS_PASSTHROUGH document Jun 11, 2024

zijun726911 requested a review from erikfuller June 11, 2024 17:44

zijun726911 commented Jun 11, 2024

View reviewed changes

zijun726911 marked this pull request as ready for review June 11, 2024 19:43

zijun726911 force-pushed the tls-passthrough-doc-2 branch from 27bd96d to 266d4a9 Compare June 11, 2024 19:47

Minor Change

112b6b0

zijun726911 force-pushed the tls-passthrough-doc-2 branch from 266d4a9 to 112b6b0 Compare June 11, 2024 20:01

Mengli-Shu reviewed Jun 12, 2024

View reviewed changes

docs/api-types/tls-route.md Outdated Show resolved Hide resolved

Mengli-Shu reviewed Jun 12, 2024

View reviewed changes

docs/guides/tls-passthrough.md Outdated Show resolved Hide resolved

Mengli-Shu reviewed Jun 12, 2024

View reviewed changes

Zijun Wang added 2 commits June 12, 2024 11:40

Address PR comments, minor change

642eed9

Minor change

15509b9

erikfuller requested changes Jun 13, 2024

View reviewed changes

Address PR comments

1dbd98c

zijun726911 force-pushed the tls-passthrough-doc-2 branch from c93826a to 1dbd98c Compare June 14, 2024 18:20

erikfuller approved these changes Jun 14, 2024

View reviewed changes

erikfuller merged commit f60df58 into aws:main Jun 14, 2024
2 checks passed

zijun726911 mentioned this pull request Jun 15, 2024

Update the Public Doc to mention the TargetGroupPolicy supporting attaching to the ServiceExport #647

Closed

zijun726911 mentioned this pull request Jul 3, 2024

Feature Parity: Support TLSRoute and VPC Lattice TLS_PASSTHROUGH feature #637

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TLS_PASSTHROUGH document #650

TLS_PASSTHROUGH document #650

zijun726911 commented Jun 10, 2024 •

edited

Loading

zijun726911 Jun 11, 2024 •

edited

Loading

Mengli-Shu Jun 12, 2024

zijun726911 Jun 12, 2024

Mengli-Shu Jun 13, 2024

zijun726911 Jun 13, 2024

erikfuller Jun 13, 2024

zijun726911 Jun 14, 2024

erikfuller Jun 14, 2024

Mengli-Shu Jun 12, 2024

zijun726911 Jun 12, 2024

erikfuller left a comment

erikfuller Jun 13, 2024

erikfuller Jun 13, 2024

erikfuller Jun 13, 2024

zijun726911 Jun 14, 2024

erikfuller Jun 14, 2024

erikfuller Jun 13, 2024

zijun726911 Jun 14, 2024

erikfuller Jun 13, 2024

zijun726911 Jun 14, 2024

erikfuller Jun 13, 2024

zijun726911 Jun 14, 2024

erikfuller left a comment

	## Example Configuration

	This will enable HTTPS traffic between the gateway and Kubernetes service, with customized health check configuration.

	```
	apiVersion: application-networking.k8s.aws/v1alpha1
	kind: TargetGroupPolicy
	metadata:
	name: test-policy
	spec:
	targetRef:
	group: ""
	kind: Service
	name: my-parking-service
	protocol: HTTPS
	protocolVersion: HTTP1
	healthCheck:
	enabled: true
	intervalSeconds: 5
	timeoutSeconds: 1
	healthyThresholdCount: 3
	unhealthyThresholdCount: 2
	path: "/healthcheck"
	port: 80
	protocol: HTTP
	protocolVersion: HTTP1
	statusMatch: "200"

TLS_PASSTHROUGH document #650

TLS_PASSTHROUGH document #650

Conversation

zijun726911 commented Jun 10, 2024 • edited Loading

Changes

Test

zijun726911 Jun 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikfuller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikfuller left a comment

Choose a reason for hiding this comment

zijun726911 commented Jun 10, 2024 •

edited

Loading

zijun726911 Jun 11, 2024 •

edited

Loading